refactor(cli): decide restores in one place - #554
Merged
Conversation
RestoreRegularFilesUseCase and RestoreMergeFilesUseCase each ran the same loop: instantiate ResolveRestoreDecisionUseCase, ask it per drift entry, partition into restored and kept. A bug in the force, interactive or skip logic had to be fixed twice. RestoreDriftEntriesUseCase now owns that loop. The parts that genuinely differ go through a RestoreDriftLeaf with three methods: collectDrift, restore, and buildResult. Composition, not inheritance: each use-case builds its own skeleton instance and hands it a leaf. No branch on which caller it is serving lives in the shared file. The I/O leaf stays two implementations, as the cartography requires. Regular still replaces the whole file via writeFile; merge still merges only the drifted keys via mergeJsonFile. Neither call appears in the shared skeleton. Construction sites for ResolveRestoreDecisionUseCase in src go from two to one. That makes the code path singular, not the runtime object: each restore use-case still holds its own skeleton instance. Both classes keep their public constructor and execute signature, so restore-tool-files-use-case.ts needed no edit. 2144/2144 pass (2136 plus 8 new merge tests), tsc clean, no cast and no any. No existing assertion changed: the restore-regular tests added last ticket were this refactor's safety net and passed unedited. Mutation-tested by inverting the skip check in the skeleton: 20 tests failed across restore-regular, restore-merge and restore-use-case, so both paths broke on one edit. Partial merge verified explicitly: with one key drifted, one undrifted, and one absent from the distribution entirely, restore syncs only the drifted key and leaves the other two untouched. --no-verify: biome OOMs here; each changed file was checked individually and reports no fixes.
blafourcade
added a commit
that referenced
this pull request
Jul 31, 2026
RestoreRegularFilesUseCase and RestoreMergeFilesUseCase each ran the same loop: instantiate ResolveRestoreDecisionUseCase, ask it per drift entry, partition into restored and kept. A bug in the force, interactive or skip logic had to be fixed twice. RestoreDriftEntriesUseCase now owns that loop. The parts that genuinely differ go through a RestoreDriftLeaf with three methods: collectDrift, restore, and buildResult. Composition, not inheritance: each use-case builds its own skeleton instance and hands it a leaf. No branch on which caller it is serving lives in the shared file. The I/O leaf stays two implementations, as the cartography requires. Regular still replaces the whole file via writeFile; merge still merges only the drifted keys via mergeJsonFile. Neither call appears in the shared skeleton. Construction sites for ResolveRestoreDecisionUseCase in src go from two to one. That makes the code path singular, not the runtime object: each restore use-case still holds its own skeleton instance. Both classes keep their public constructor and execute signature, so restore-tool-files-use-case.ts needed no edit. 2144/2144 pass (2136 plus 8 new merge tests), tsc clean, no cast and no any. No existing assertion changed: the restore-regular tests added last ticket were this refactor's safety net and passed unedited. Mutation-tested by inverting the skip check in the skeleton: 20 tests failed across restore-regular, restore-merge and restore-use-case, so both paths broke on one edit. Partial merge verified explicitly: with one key drifted, one undrifted, and one absent from the distribution entirely, restore syncs only the drifted key and leaves the other two untouched. --no-verify: biome OOMs here; each changed file was checked individually and reports no fixes.
blafourcade
added a commit
that referenced
this pull request
Jul 31, 2026
RestoreRegularFilesUseCase and RestoreMergeFilesUseCase each ran the same loop: instantiate ResolveRestoreDecisionUseCase, ask it per drift entry, partition into restored and kept. A bug in the force, interactive or skip logic had to be fixed twice. RestoreDriftEntriesUseCase now owns that loop. The parts that genuinely differ go through a RestoreDriftLeaf with three methods: collectDrift, restore, and buildResult. Composition, not inheritance: each use-case builds its own skeleton instance and hands it a leaf. No branch on which caller it is serving lives in the shared file. The I/O leaf stays two implementations, as the cartography requires. Regular still replaces the whole file via writeFile; merge still merges only the drifted keys via mergeJsonFile. Neither call appears in the shared skeleton. Construction sites for ResolveRestoreDecisionUseCase in src go from two to one. That makes the code path singular, not the runtime object: each restore use-case still holds its own skeleton instance. Both classes keep their public constructor and execute signature, so restore-tool-files-use-case.ts needed no edit. 2144/2144 pass (2136 plus 8 new merge tests), tsc clean, no cast and no any. No existing assertion changed: the restore-regular tests added last ticket were this refactor's safety net and passed unedited. Mutation-tested by inverting the skip check in the skeleton: 20 tests failed across restore-regular, restore-merge and restore-use-case, so both paths broke on one edit. Partial merge verified explicitly: with one key drifted, one undrifted, and one absent from the distribution entirely, restore syncs only the drifted key and leaves the other two untouched. --no-verify: biome OOMs here; each changed file was checked individually and reports no fixes.
Merged
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
🎯 What & why
RestoreRegularFilesUseCaseandRestoreMergeFilesUseCaseeach ran the same loop (item B4): instantiateResolveRestoreDecisionUseCase, ask it per drift entry, partition into restored and kept. A bug in the force / interactive / skip logic had to be fixed twice.🛠️ How it works
RestoreDriftEntriesUseCasenow owns that loop. The parts that genuinely differ go through aRestoreDriftLeaf<TDrift, TResult>with three methods:collectDrift,restore,buildResult.Composition, not inheritance — each use-case builds its own skeleton instance and hands it a leaf, per the note on the previous PR. No branch on which caller it is serving lives in the shared file.
Construction sites for
ResolveRestoreDecisionUseCaseinsrc/go from two to one. That makes the code path singular, not the runtime object: each restore use-case still holds its own skeleton instance.Both classes keep their public constructor and
executesignature, sorestore-tool-files-use-case.tsneeded no edit.The cartography confirms the difference at the I/O leaf is legitimate, and it survives:
fs.writeFilefs.mergeJsonFileNeither call appears in the shared skeleton.
Verified explicitly by a new test: with one key drifted, one undrifted, and one absent from the framework distribution entirely, restore syncs only the drifted key and leaves the other two untouched. A whole-file replacement would have destroyed both.
🧪 How to verify
2144/2144 pass (2136 plus 8 new merge tests),
tsc --noEmitclean, no cast and noany.No existing assertion changed — the
restore-regulartests added in #550 (which took that file from 42.7% to 100% statements) were written as this refactor's safety net and passed unedited.Mutation-tested by inverting the skip check in the skeleton: 20 tests failed across
restore-regular,restore-mergeandrestore-use-case— both paths broke on one edit, which is the property the extraction is meant to have.This completes epic E8, and with it the whole cartography backlog.
Committed with
--no-verify: biome OOMs on this machine; each changed file was checked individually and reports no fixes.